Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Searchtools: don't assume that all themes define some elements #10153

Merged
merged 2 commits into from
Apr 4, 2022

Conversation

stsewd
Copy link
Contributor

@stsewd stsewd commented Feb 1, 2022

Feature or Bugfix

  • Bugfix

Purpose

When retrieving a non-existent element, jQuery would still return an
object (kind of empty one, so method calls won't raise a null
exception), but now getElementById will return null and raise an
exception when trying to call a method on that value.

This mainly affects the rtd theme,
which completely overrides the search page
https://github.com/readthedocs/sphinx_rtd_theme/blob/d64dadf1ceec4f9ff6c1ca2d3ea4c3d0fdb0e8d2/sphinx_rtd_theme/search.html

Relates

When retrieving a non-existent element, jQuery would still return an
object (kind of empty one, so method calls won't raise a null
exception), but now `getElementById` will return null and raise an
exception when trying to call a method on that value.

This mainly affects the rtd theme,
which completely overrides the search page
https://github.com/readthedocs/sphinx_rtd_theme/blob/d64dadf1ceec4f9ff6c1ca2d3ea4c3d0fdb0e8d2/sphinx_rtd_theme/search.html
@tk0miya tk0miya added this to the 5.0.0 milestone Feb 6, 2022
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
stsewd added a commit to readthedocs/readthedocs.org that referenced this pull request Mar 3, 2022
stsewd added a commit to readthedocs/readthedocs.org that referenced this pull request Mar 3, 2022
stsewd added a commit to readthedocs/readthedocs.org that referenced this pull request Mar 3, 2022
@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

A small note that Search is still broken for me with RTD theme with this pull request:

searchtools.js:232 Uncaught TypeError: Cannot read properties of null (reading 'toLowerCase')
    at Object.query (searchtools.js:232:39)
    at Object.setIndex (searchtools.js:171:14)
    at searchindex.js:1:8

@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

A small note that Search is still broken for me with RTD theme with this pull request:

searchtools.js:232 Uncaught TypeError: Cannot read properties of null (reading 'toLowerCase')
    at Object.query (searchtools.js:232:39)
    at Object.setIndex (searchtools.js:171:14)
    at searchindex.js:1:8

Which is related to intersphinx:

Search.setIndex({docnames:["demo","demo2","index","test/source/index"],envversion:{"sphinx.domains.c":2,"sphinx.domains.changeset":1,"sphinx.domains.citation":1,"sphinx.domains.cpp":4,"sphinx.domains.index":1,"sphinx.domains.javascript":2,"sphinx.domains.math":2,"sphinx.domains.python":3,"sphinx.domains.rst":2,"sphinx.domains.std":2,"sphinx.ext.intersphinx":1,sphinx:56},filenames:["demo.rst","demo2.rst","index.rst","test/source/index.rst"],objects:{"":[[1,0,1,"_CPPv412__bid_adddd310_Decimal6410_Decimal64","__bid_adddd3"],[1,1,1,"_CPPv412__bid_adddd310_Decimal6410_Decimal64","__bid_adddd3::a"],[1,1,1,"_CPPv412__bid_adddd310_Decimal6410_Decimal64","__bid_adddd3::b"],[1,0,1,"_CPPv412__bid_addsd310_Decimal3210_Decimal32","__bid_addsd3"],[1,1,1,"_CPPv412__bid_addsd310_Decimal3210_Decimal32","__bid_addsd3::a"],[1,1,1,"_CPPv412__bid_addsd310_Decimal3210_Decimal32","__bid_addsd3::b"],[1,0,1,"_CPPv412__bid_addtd311_Decimal12811_Decimal128","__bid_addtd3"],[1,1,1,"_CPPv412__bid_addtd311_Decimal12811_Decimal128","__bid_addtd3::a"],[1,1,1,"_CPPv412__bid_addtd311_Decimal12811_Decimal128","__bid_addtd3::b"],[1,0,1,"_CPPv412__dpd_adddd310_Decimal6410_Decimal64","__dpd_adddd3"],[1,1,1,"_CPPv412__dpd_adddd310_Decimal6410_Decimal64","__dpd_adddd3::a"],[1,1,1,"_CPPv412__dpd_adddd310_Decimal6410_Decimal64","__dpd_adddd3::b"],[1,0,1,"_CPPv412__dpd_addsd310_Decimal3210_Decimal32","__dpd_addsd3"],[1,1,1,"_CPPv412__dpd_addsd310_Decimal3210_Decimal32","__dpd_addsd3::a"],[1,1,1,"_CPPv412__dpd_addsd310_Decimal3210_Decimal32","__dpd_addsd3::b"],[1,0,1,"_CPPv412__dpd_addtd311_Decimal12811_Decimal128","__dpd_addtd3"],[1,1,1,"_CPPv412__dpd_addtd311_Decimal12811_Decimal128","__dpd_addtd3::a"],[1,1,1,"_CPPv412__dpd_addtd311_Decimal12811_Decimal128","__dpd_addtd3::b"],[1,0,1,"_CPPv411gimple_code6gimple","gimple_code"],[1,1,1,"_CPPv411gimple_code6gimple","gimple_code::g"],[1,2,1,"cmdoption-o","--object-directory"],[1,2,1,"cmdoption-o","--object-file"],[0,2,1,"cmdoption-Wall","-Wall"],[0,2,1,"cmdoption-Wauggest-attribute","-Wauggest-attribute"],[0,2,1,"cmdoption-Wno-shift-overflow","-Wno-shift-overflow"],[0,2,1,"cmdoption-Wno-shift-overflow2","-Wno-shift-overflow2"],[0,2,1,"cmdoption-Wno-shift-overflow3","-Wno-shift-overflow3"],[0,2,1,"cmdoption-Wno-shift-overflow","-Wshift-overflow"],[0,2,1,"cmdoption-Wno-shift-overflow2","-Wshift-overflow2"],[0,2,1,"cmdoption-Wshift-overflow3","-Wshift-overflow3"],[1,2,1,"cmdoption-foo","-f"],[1,2,1,"cmdoption-foo","-foo"],[1,2,1,"cmdoption-mmmx","-mmmx"],[1,2,1,"cmdoption-msse","-msse"],[1,2,1,"cmdoption-msse2","-msse2"],[1,2,1,"cmdoption-o","-o"]],make:[[1,2,1,"cmdoption-make-verbose","--verbose"]]},objnames:{"0":["cpp","function","C++ function"],"1":["cpp","functionParam","C++ function parameter"],"2":["std","cmdoption","program option"]},objtypes:{"0":"cpp:function","1":"cpp:functionParam","2":"std:cmdoption"},terms:{"0":[0,1,2,3],"1":[0,1,2,3],"12":[0,1,2,3],"2":[0,1],"3":0,"4":1,"6":1,"69":1,"byte":0,"const":0,"default":0,"do":0,"enum":1,"function":2,"karl\u00ed\u010dek":0,"ko\u0144\u00ed\u010dku":0,"null":0,"public":1,"return":1,"static":0,"super":0,"switch":1,"void":0,And:1,These:[0,1],__attribute__:0,__bid_adddd3:1,__bid_addsd3:1,__bid_addtd3:1,__dpd_adddd3:1,__dpd_addsd3:1,__dpd_addtd3:1,_decimal128:1,_decimal32:1,_decimal64:1,about:0,ac:1,access:1,acloc:1,ada:1,after:0,all:[0,1],alloc:0,also:1,alwai:0,am:[0,1],analyz:0,ani:1,ar:1,argument:0,arithmet:2,arrai:0,attribut:0,auto:0,autoconf:1,avail:1,b:1,bar:1,baz:1,block:[0,1],both:1,buffer:0,bug:[0,1,2,3],build:2,c:[0,1],call:0,callgraph:0,capit:1,caret:0,cc:1,chang:0,checker:0,code:[0,1],cold:0,color:0,column:0,command:0,complex:0,config:1,configur:1,control:0,count:0,cp:1,cpp:1,cwe:0,cxx:1,dealloc:0,defin:0,demo:0,depth:0,derefer:0,dest:0,develop:1,diagnost:0,directori:1,displai:0,divisor:0,document:0,doubl:0,edg:0,elid:0,els:1,emphasi:0,enabl:[0,1],error:0,escap:0,etc:1,event:0,everi:0,exit:0,explod:0,exposur:0,extend:0,extens:0,extern:0,f:1,fanalyz:0,fatal:0,fclose:0,fdiagnost:0,fdump:0,feasibl:0,file:[0,1],fine:0,fixit:0,fmessag:0,fno:0,foo:1,foobar:1,format:0,frame:0,franc:0,free:0,futur:0,g:1,gcc:[0,2,3],gener:0,gimpl:1,gimple_cod:1,git:1,gnu:[0,1,2,3],grain:0,graph:0,h:1,handler:0,header:1,heap:0,hh:1,hp:1,hpp:1,http:[0,1,2,3],hxx:1,i:[0,1],index:[0,2,3],inlin:0,instal:1,instruct:1,introduct:2,json:0,label:0,last:1,later:1,leak:0,left:0,len:0,length:0,letter:1,level:0,likewis:1,line:0,list:1,liter:[0,1],locat:0,m4:1,m:[0,1],mai:0,malloc:0,margin:0,merg:0,messag:0,mii:1,minimum:0,mismatch:0,mm:1,mmmx:1,mmx:1,mode:1,modifi:1,modul:3,msse2:1,msse:1,multipl:2,must:1,my:0,my_memcpi:0,n:0,name:[0,2],necessari:2,neg:0,never:0,node:0,non:0,none:0,nonnul:0,noreturn:0,note1:0,note2:0,note:[0,1],number:0,o:1,object:1,offset:0,oi:0,onc:0,option:[0,1,3],org:[0,1,2,3],origin:0,out:[0,1],output:0,overflow2:0,overflow3:[0,1,3],overflow:0,packag:2,page:[0,3],parseabl:0,patch:0,path:0,pattern:0,plain:0,pointer:0,possibl:0,precompil:1,preprocess:1,print:0,ptr_extend:0,pure:0,purg:0,refer:1,regener:1,releas:1,reli:0,repositori:1,requir:1,s:1,samp:0,search:3,see:0,separ:0,setjmp:0,shift:[0,1,3],should:1,show:0,sign:0,signal:0,size:0,size_t:0,snapshot:1,someth:1,sourc:1,spec:1,src:0,sse2:1,sse:1,ssh:1,stack:0,stale:0,state:0,statement:1,stderr:0,string:0,strong:0,suggest:0,sum:1,summari:0,supergraph:0,taint:0,target:0,tcc:1,templat:0,test:1,text:0,thi:[0,1],through:0,too:0,tool:2,transit:0,tree:0,turn:1,two:1,type:0,unicod:0,uniniti:0,unit:0,unsaf:0,url:0,us:[0,1,3],valu:0,variabl:0,verbos:[0,1],version:[0,1],via:1,wall:0,wanalyz:0,warn:0,wauggest:0,weekli:1,when:1,width:0,within:0,wno:[0,1,3],work:1,write:0,wshift:0,x:1,xavier:0,zero:0},titles:["Introduction","Multiple","Demo documentation","Welcome to test\u2019s documentation!"],titleterms:{"function":1,arithmet:1,build:1,demo:2,document:[2,3],gcc:1,indic:3,introduct:0,multipl:1,name:1,necessari:1,packag:1,s:3,tabl:3,test:3,tool:1,welcom:3}})

@jakobandersen Should I create a separate issue for it?

@AA-Turner
Copy link
Member

AA-Turner commented Mar 7, 2022

That traceback doesn't make sense to me -- .query is being called with null, but .setIndex explicitly checks that what it passes to .query is not null:

if (Search._queued_query !== null) {
const query = Search._queued_query;
Search._queued_query = null;
Search.query(query);
}

A

@AA-Turner
Copy link
Member

@marxin do you have a full minimal reproducer you could share?

A

@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

@marxin do you have a full minimal reproducer you could share?

I have a fix, moment.

@marxin
Copy link
Contributor

marxin commented Mar 7, 2022

It got fixed with 0a36a47 which is not included stsewd:check-if-search-progress-exists branch, that's why I met the issue.

@marxin marxin mentioned this pull request Mar 7, 2022
25 tasks
@tk0miya tk0miya changed the base branch from master to 5.x April 2, 2022 18:10
@tk0miya
Copy link
Member

tk0miya commented Apr 2, 2022

May I merge this?

@stsewd
Copy link
Contributor Author

stsewd commented Apr 4, 2022

May I merge this?

From my side yes, and looks like this was approved too #10153 (review)

@tk0miya
Copy link
Member

tk0miya commented Apr 4, 2022

Okay, merging this now. Thank you for your contribution!

@tk0miya tk0miya merged commit b4276ed into sphinx-doc:5.x Apr 4, 2022
@stsewd stsewd deleted the check-if-search-progress-exists branch April 4, 2022 19:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants